[6/n] [sled-agent] add remove_mupdate_override to OmicronSledConfig and friends #8097
Conversation
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
sunshowers
changed the title
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
| #[derive(Debug, Args)] | ||
| struct SledSetPolicyArgs { | ||
| /// id of the sled | ||
| sled_id: SledUuid, | ||
|
|
||
| /// The policy to set for the sled | ||
| #[clap(value_enum)] | ||
| policy: SledPolicyOpt, | ||
| } | ||
|
|
||
| #[derive(Clone, Copy, Debug, ValueEnum)] | ||
| enum SledPolicyOpt { | ||
| InService, | ||
| NonProvisionable, | ||
| Expunged, | ||
| } | ||
|
|
||
| impl fmt::Display for SledPolicyOpt { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| match self { | ||
| SledPolicyOpt::InService => write!(f, "in-service"), | ||
| SledPolicyOpt::NonProvisionable => write!(f, "non-provisionable"), | ||
| SledPolicyOpt::Expunged => write!(f, "expunged"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl From<SledPolicyOpt> for SledPolicy { | ||
| fn from(value: SledPolicyOpt) -> Self { | ||
| match value { | ||
| SledPolicyOpt::InService => SledPolicy::InService { | ||
| provision_policy: SledProvisionPolicy::Provisionable, | ||
| }, | ||
| SledPolicyOpt::NonProvisionable => SledPolicy::InService { | ||
| provision_policy: SledProvisionPolicy::NonProvisionable, | ||
| }, | ||
| SledPolicyOpt::Expunged => SledPolicy::Expunged, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I wrote this but honestly didn't end up needing it -- left it in here because it seems somewhat useful.
Created using spr 1.3.6-beta.1 [skip ci]
| > # Load example system with 7 sleds: | ||
| > # | ||
| > # sled 0: unset -> unset (unchanged) | ||
| > # sled 1: unset -> set | ||
| > # sled 2: set -> unset | ||
| > # sled 3: set -> set (unchanged) | ||
| > # sled 4: set -> set (changed) | ||
| > # sled 5: set -> set (unchanged) but change something else | ||
| > # sled 6: set -> sled removed | ||
| > # | ||
| > # We'll also add another sled below (new_sled_id) with | ||
| > # remove_mupdate_override set. | ||
| > # | ||
| > # We don't need any zones for this test, so disable that to keep the | ||
| > # outputs minimal. |
There was a problem hiding this comment.
#8142 makes this much easier to read.
| }, | ||
| "remove_mupdate_override": null |
There was a problem hiding this comment.
Are there backcompat considerations here?
There was a problem hiding this comment.
Definitely not yet - nothing has landed or shipped that would ledger these merged configs on deployed systems. (Even if we had, I think the answer would still be no - this is confirming that we can convert the legacy configs to the new config, and getting null here is exactly what we want.)
|
|
||
| /// Debug method to remove a sled from a blueprint entirely. | ||
| /// | ||
| /// Bypasses all expungement checks. Do not use in production. |
There was a problem hiding this comment.
Do you think the name + comment is enough, or should we put this behind a cargo feature? (Not a leading question; I'm genuinely not sure)
There was a problem hiding this comment.
I tend to avoid Cargo features in general because of duplicated builds with and without the feature, though in some cases Cargo features are unavoidable. Here, I think the debug_ name is enough personally.
| }, | ||
| "remove_mupdate_override": null |
There was a problem hiding this comment.
Definitely not yet - nothing has landed or shipped that would ledger these merged configs on deployed systems. (Even if we had, I think the answer would still be no - this is confirming that we can convert the legacy configs to the new config, and getting null here is exactly what we want.)
| load-example --nsleds 7 --ndisks-per-sled 0 --no-zones | ||
|
|
||
| # Set the field on sleds 2-6 (0-indexed). | ||
| blueprint-edit latest set-remove-mupdate-override 9a867dc9-d505-427f-9eff-cdb1d4d9bd73 00000000-0000-0000-0000-000000000000 |
There was a problem hiding this comment.
Just out of curiosity - what was your workflow for figuring out what IDs correspond to which sleds as you wrote this test?
There was a problem hiding this comment.
First showed the blueprint for the example system in the output, and grabbed the list of UUIDs from there. I'd love to add more symbolic names at some point.
sunshowers
changed the base branch from
sunshowers/spr/main.sled-agent-add-remove_mupdate_override-to-omicronsledconfig-and-friends
to
main
sunshowers
deleted the
sunshowers/spr/sled-agent-add-remove_mupdate_override-to-omicronsledconfig-and-friends
branch
This adds:
In subsequent commits we'll make decisions based on this.
Depends on: